Why it's not always a boolean?

If I'm understanding properly, the check is less about type "boolean" and more about checking if the value is actually set to avoid considering "undefined" as "false".

I think I'd prefer if we stay consistent between all the default controls. We're considering undefined as false for all the default controls in all the *Panel components. So my question is: can we just keep it as is here? and update the "block.json" files for the blocks that override the default defaultControls instead?

If I'm understanding properly, the check is less about type "boolean" and more about checking if the value is actually set to avoid considering "undefined" as "false".

Yes, that's the point of this check.

We're considering undefined as false for all the default controls in all the *Panel components. So my question is: can we just keep it as is here? and update the "block.json" files for the blocks that override the default defaultControls instead?

That's exactly what this change addresses: we need to have a way of defaulting to true for the child layout controls, but still preserving the ability to override that if needed by setting it to false in block.json. Child layout controls can be applied to any block, including third party blocks, so it's not practical to explicitly set the controls to display for every single block type.

I like the idea of exposing false as a valid value, and the idea that we can have some controls be opt-out (in terms of the default controls) rather than opt-in.

If consistency for each of the controls is important, what if we used something like isShownByDefault={ defaultControls.childLayout ?? FALLBACK_DEFAULT_CONTROLS.childLayout } for each of the controls. Then, FALLBACK_DEFAULT_CONTROLS could contain the global defaults we'd like to go with. So if all the others are hidden by default, it might be:

const FALLBACK_DEFAULT_CONTROLS = {
	...
	padding: false,
	margin: false,
	blockGap: false,
	minHeight: false,
	childLayout: true,
};

Would something like that work? The idea's basically the same as this PR, but just that we could use the same approach as this PR for each of the other controls, with a const to contain our desired defaults.

isShownByDefault={ defaultControls.childLayout ?? FALLBACK_DEFAULT_CONTROLS.childLayout }

This works for me 👍

Thanks for the suggestion! I'll give it a go. Note that doing that will also require us to explicitly set all the controls to true on the site editor side, given that currently the site editor isn't passing any defaultControls into the Dimensions panel, so it depends on the values from DEFAULT_CONTROLS to be true.

Ok I've added the suggested changes!

I'm sorry I don't understand why we had to change the default values here? Yeah, there's a small potential of changing the UI for existing blocks, but for me "true" makes more sense as default values for all of these.

In other words, a user of DimensionsControl shouldn't have to define the "defaultControls" prop in order to have a working component.

If you're worried about the changes for blocks, we can just update the block.json of the affected blocks?

The other piece of feedback I have is that we should be consistent in behavior between all the *Panel component (we have 4 right now I believe)

Any thoughts on this? I'm actually not really satisfied that this PR changed the behavior of default controls in one Styles UI component but not the others, I'm also not convinced that this is the right behavior. Global styles are not customizable by the user, so they shouldn't need to pass any "defaultControls" prop, while the "blocks" (block inspector) are the ones that can customize the default controls, so they should be the ones providing a custom value for the defaultControls prop.

It's a good question @youknowriad. When reviewing this one, I quite liked the idea of the panel itself defining the defaults, and then the site editor/global styles as an exception specifying that it wants to display all the controls. In that way, the defaults at the component level follow the expected behaviour in the block editor. But I can see the goal in preferencing things one way or the other.

Either way, I agree that it'd be good to make sure it's consistent across the different panels. If we're to do that, do you have a preference for how to proceed while maintaining displaying the child layout controls by default?

I've also left a similar comment on the styles UI documentation PR (#49720 (comment)) so that these discussions are connected.

The main reason I went with this option was to not change things for third party blocks. Previously, the default was none of the controls would display except for global styles, but the default setting for the controls was to display all. Blocks outside of global styles weren't displaying controls because the defaults were being silently overridden by an empty object, which is non-ideal. But that's how things have been for a while, and if we change the default behaviour now, any custom blocks using these block supports will suddenly have a bunch of controls displaying by default that they didn't have before.

Personally, I'd love to have everything display by default, because having to look for controls in dropdowns is excruciatingly painful UX, but I'm concerned about changing things around for extenders with no warning.

I'm happy to make the same change for the other block supports as long as we're clear this is the way we want to go.

I agree that it's not idea to change behavior on block authors but maybe we can make some small adjustments as it's not entirely a breaking change, things still work, they may just be hidden or visible. I think a dev note in this case would be cool.

Maybe we should try to make the change with the least disruption to block authors while making sure the default values are decent and consistent across the components. what would that be?

making sure the default values are decent and consistent across the components. what would that be?

I'd be happy to make them all visible by default all the time, but perhaps best defer to @WordPress/gutenberg-design 😄